Skip to content

chore: regenerate test vectors against leanSpec main and fix fork-choice harness - #120

Merged
ArtiomTr merged 4 commits into
grandinetech:devnet-5from
SoarinSkySagar:test-vectors-update
Jul 30, 2026
Merged

chore: regenerate test vectors against leanSpec main and fix fork-choice harness#120
ArtiomTr merged 4 commits into
grandinetech:devnet-5from
SoarinSkySagar:test-vectors-update

Conversation

@SoarinSkySagar

Copy link
Copy Markdown

used make generate-test-vectors LEAN_SPEC_COMMIT=0f5b8e58e58c0e155ae8b95eabbbde0b9ad8840a to download the latest leanSpec tests and fixed the harness for fork_choice, the previous test suite had fc in the directory but new suite has fork_choice, so had to to change it.

@ArtiomTr ArtiomTr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good, only minor nitpicks need to be fixed

Comment thread lean_client/spec_test_fixtures/src/common.rs Outdated
Comment thread lean_client/Makefile Outdated
Comment thread lean_client/spec_test_fixtures/src/fork_choice.rs Outdated
Comment thread lean_client/fork_choice/tests/fork_choice_test_vectors.rs Outdated
@SoarinSkySagar

Copy link
Copy Markdown
Author

@ArtiomTr all required changes done!

@SoarinSkySagar
SoarinSkySagar requested a review from ArtiomTr July 29, 2026 14:20

@ArtiomTr ArtiomTr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 👍

@ArtiomTr

Copy link
Copy Markdown
Collaborator

looks like client build fails? something with http_api test driver:

error[E0609]: no field `proof_data` on type `GossipProofJSON`
   --> http_api/src/test_driver.rs:463:32
    |
463 |     let proof_hex = step.proof.proof_data.data.trim_start_matches("0x");
    |                                ^^^^^^^^^^ unknown field
    |

@SoarinSkySagar

SoarinSkySagar commented Jul 29, 2026

Copy link
Copy Markdown
Author

looks like client build fails? something with http_api test driver:

error[E0609]: no field `proof_data` on type `GossipProofJSON`
   --> http_api/src/test_driver.rs:463:32
    |
463 |     let proof_hex = step.proof.proof_data.data.trim_start_matches("0x");
    |                                ^^^^^^^^^^ unknown field
    |

yeah sorry about that, I forgot to check build before committing. should be fine now.

@SoarinSkySagar
SoarinSkySagar requested a review from ArtiomTr July 29, 2026 20:24
@Sahilgill24

Copy link
Copy Markdown

hey @ArtiomTr @SoarinSkySagar , I was just in general looking through this and is there a reason why you guys are directly adding the JSON fixtures to the repository.
I think using the leanspec fixtures release in the Makefile and then downloading it on the local for testing would be better than adding the fixtures directly to the repo and then changing them again and again based on the fixture changes.
I have written a small review for the make file that would perform this task.

@Sahilgill24 Sahilgill24 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like this and now the fixtures are in the local and test's can be run on it, I believe all the other lean clients follow this only.

LEAN_SPEC_FIXTURES_URL ?= https://github.com/leanEthereum/leanSpec/releases/latest/download/fixtures-prod-scheme.tar.gz

fixtures-temp:
	tmpdir=$$(mktemp -d); \
	echo "Extracting fixtures to $$tmpdir"; \
	curl -L -f "$(LEAN_SPEC_FIXTURES_URL)" | tar -xz -C "$$tmpdir"; \
	echo "Fixtures available at: $$tmpdir"

@SoarinSkySagar

SoarinSkySagar commented Jul 30, 2026

Copy link
Copy Markdown
Author

is there a reason why you guys are directly adding the JSON fixtures to the repository

@Sahilgill24 I think the main concern we have with this approach is that it takes a very long time just to regenerate the test vectors (~57 minutes for all tests and total 2.5 hours with deterministic tests, on my machine). So regenerating the test vectors each time during testing consumes a lot of time when we can just download and reuse them and only regenerate them when we have a major update on the tests.

Would like to hear your suggestions/opinion on this.

@Sahilgill24

Copy link
Copy Markdown

I am not sure why it would be taking 57 minutes for the regeneration because the tar archive for the test vectors is just 150Mb and running them is like almost 1-2 minutes.
are you surely running them by downloading the fixtures from the releases right? rather than using the uv sync command on the leanspec repo, because I think that can take so much time

@ArtiomTr

Copy link
Copy Markdown
Collaborator

hey @ArtiomTr @SoarinSkySagar , I was just in general looking through this and is there a reason why you guys are directly adding the JSON fixtures to the repository.

Because we don't want to make our CI builds fragile. See, with your proposed script, you're downloading "latest" release. This makes CI impure, and this brings tons of issues to development process:

  • if I want to debug test failure, how do I make sure my CI downloads exact same version as I do?
  • if someone accidentally breaks test vectors / pushes incorrect test vectors into lean spec, how can I revert those changes to one version back?

This also makes some things more irritating, though they don't affect development directly:

  • re-running CI may pass/fail on previously failing/passing commit;
  • CI on old branches (like devnet-2 for example) will fail, if we try to re-run tests.

Some good things that we get from our approach, which are nice-to-have, but not the main reason we do things such way:

  • git checkout <branch> automatically pulls correct version of tests - this means I can easily run cargo test -p fork_choice for example, and never see some obscure errors, something like glob pattern "test_vectors/fork_choice/*/fc/*/*.json" didn't match any files! or just randomly failing tests, because I didn't ran make test, that automatically pulls correct version of test vectors;
  • the "pulling correct version of tests" thing also means that I can run tests granularly, what especially comes handy with AI agents - not needing to run whole test suite makes iterating faster;
  • nothing stops us from post-processing (e.g., reshaping) or changing test vectors by hand. We haven't tried doing that, but it may be handy in some cases.

Our approach has some downsides too:

  • test vectors pollute history, git database, and I think probably slow down some things, like checkouts;
  • we have to not forget to sync our tests with spec version, otherwise we may fix issues that aren't present in latest spec version.

With that all said, I think our current approach is better than downloading latest spec test version. Maybe if leanSpec does tagged releases at some point, then we could transition to downloading spec tests instead of generating them on our own - though I think currently that doesn't make much sense for leanSpec, considering the pace new changes are introduced.

@ArtiomTr
ArtiomTr merged commit bcfe81c into grandinetech:devnet-5 Jul 30, 2026
2 of 3 checks passed
@ArtiomTr

Copy link
Copy Markdown
Collaborator

@Sahilgill24 if you want to continue discussion, I can create a separate issue for that, so we can discuss other approaches of managing test vectors.

@Sahilgill24

Copy link
Copy Markdown

@ArtiomTr surely, we can discuss a few approaches

@SoarinSkySagar

Copy link
Copy Markdown
Author

I am not sure why it would be taking 57 minutes for the regeneration because the tar archive for the test vectors is just 150Mb and running them is like almost 1-2 minutes. are you surely running them by downloading the fixtures from the releases right? rather than using the uv sync command on the leanspec repo, because I think that can take so much time

yes, I was using the uv command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants